Add top level attributes and commands to simplify API#82
Add top level attributes and commands to simplify API#82
Conversation
GDYendell
force-pushed
the
top-level-attributes
branch
2 times, most recently
from
91267b2 to
68b31e4
Compare
Update to fastcs-odin 0.8.0a1
GDYendell
force-pushed
the
top-level-attributes
branch
from
68b31e4 to
1979b0c
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #82 +/- ##
==========================================
- Coverage 84.86% 82.00% -2.87%
==========================================
Files 13 14 +1
Lines 403 450 +47
==========================================
+ Hits 342 369 +27
- Misses 61 81 +20 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
shihab-dls
left a comment
shihab-dls
left a comment
There was a problem hiding this comment.
Looks good! Left a couple of comments that are just enquiries; Will request changes just so that you can rerequest a review when codecov is satisfied.
| TimeoutError: If parameters are not synchronised or arm PUT request fails | ||
|
|
||
| """ | ||
| await self.stale_parameters.wait_for_value(False, timeout=1) |
There was a problem hiding this comment.
Could: Now that we have wait_for_value with a timeout should we expose a DEFAULT_TIMEOUT in FastCS, such that we can ensure observation timeouts are consistent unless they explicitly shouldn't be?
There was a problem hiding this comment.
I am not sure there is a widely useful default - what else would you use that default for?
I am actually not sure if 1 second here is sufficient. Perhaps it should be an Attribute so it can be changed at runtime?
There was a problem hiding this comment.
That's fair; ophyd_async has a DEFAULT_TIMEOUT of 10s, which is used in lots of places. In this case, since wait_for_value is the only place we'd really use a default timeout, maybe just a default argument in:
async def wait_for_value(self, target_value: DType_T, *, timeout: float):for timeout?
Making it an attribute is interesting, but I feel as though users would just want a long enough timeout to realistically prompt the "is there something wrong with the ioc/device" question, similar to how timing out on an ophyd_async signal.set() prompts a similar question. So, I would lean towards the default argument in FastCS, but I'm happy with either approach.
2 participants
Uh oh!
There was an error while loading. Please reload this page.